-
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adding button icon to design system react #387
Conversation
packages/design-system-react/src/components/button-icon/ButtonIcon.constants.ts
Outdated
Show resolved
Hide resolved
packages/design-system-react/src/components/button-icon/ButtonIcon.constants.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved but lets not merge this until design is 100% done
isFloating && [ | ||
'rounded-full', | ||
!isInverse && 'bg-icon-default text-background-default', | ||
isInverse && 'text-icon-default bg-background-default', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default isInverse
+ isFloating
to the isFloating default
isInverse && 'text-icon-default bg-background-default', | |
isInverse && 'text-icon-default bg-background-default', |
0a225ce
to
dfffc10
Compare
isFloating && [ | ||
'rounded-full', | ||
!isInverse && 'bg-icon-default text-background-default', | ||
isInverse && 'bg-icon-default text-background-default', | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continue with no hover for isFloating
for now
@@ -27,8 +27,11 @@ export type { ButtonSecondaryProps } from './button-secondary'; | |||
export { ButtonTertiary, ButtonTertiarySize } from './button-tertiary'; | |||
export type { ButtonTertiaryProps } from './button-tertiary'; | |||
|
|||
export { Button, ButtonVariant } from './button'; | |||
export { Button, ButtonSize, ButtonVariant } from './button'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding missing ButtonSize
enum
!isInverse && 'bg-icon-default text-background-default', | ||
isInverse && 'bg-icon-default text-background-default', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
05159b4
to
01f3a09
Compare
It's good that you add a default aria label, but I wouldn't call that done since the string needs to be translated for localization |
Description
This PR adds
ButtonIcon
component to the design system react.Key features include:
The component is built on top of our existing ButtonBase component and integrates with our Icon system.
Related issues
Fixes: #387
Manual testing steps
yarn storybook
)Screenshots/Recordings
Before
N/A - New component
After
after720.mov
Pre-merge author checklist
Pre-merge reviewer checklist